-
Notifications
You must be signed in to change notification settings - Fork 7.3k
am243x_evm/am2434: initial support #87321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
1d084f5
to
c2c270b
Compare
bee4707
to
0e3f8d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the initial Cortex-R5F support. I looked at the changes for it and the main domain and found a few possible improvements that are in the single comments.
Also the NUM_IRQS
and SYS_CLOCK_HW_CYCLES_PER_SEC
Kconfig options need to be adjusted for this SoC. But to keep the scope of this PR limited I would suggest changing this later by splitting up the Kconfig into multiple files in a different PR.
Can you also share how you are booting the SoC and which changes you have done to the TI MCU+ SDK SBL, if you use it?
Hello @m-braunschweig
noted
I actually saw your documentation from the patchset here a few days ago: TexasInstruments#1 and I must say I am not doing it much differently. We are primarily using
For now there is also the need to set the timer clock source manually if I am using a timer different from the SBL. I appreciate your review and suggestions and will get to working on them tomorrow, might also edit this comment as I remember things. |
Ok. Did you do any modifications for using |
For R5, did nothing, the vector table at the start of SRAM (0x70000000) gets relocated to 0x0 via z_arm_relocate_vector_table For M4, you need to set the CONFIG_KERNEL_ENTRY="__start" since that is how it's configured in SBL |
The relocation itself wasn't the problem in my case. It was either parsing or setting the |
|
||
/delete-node/ &mbox4; | ||
/delete-node/ &mbox5; | ||
/delete-node/ &mbox7; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be disabled already, do you really need to delete them also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are correct but I just deleted them since they cannot be used at all by r5fss0_0
#include <zephyr/arch/arm/mpu/arm_mpu.h> | ||
|
||
static const struct arm_mpu_region mpu_regions[] = { | ||
#if defined CONFIG_SOC_AM2434_R5F0_0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use the more generic SOC_SERIES_AM6X_R5
here? Otherwise you will need to add this same check for all 4 R5F cores when those are added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a valid issue, right now there is no good way to distinguish between different cores since they are essentially seen as different "socs". Im not sure if all am6x r5 devices should have this mpu configuration We will need more discussion surrounding this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@glneo I was thinking that perhaps we could split the Kconfig for different SoCs in soc/
directory, that would be cleaner imo and would also give us freedom to define to define an option like CONFIG_SOC_AM2434_R5
which would be selected for all cores. Currently the closest thing to this is CONFIG_SOC
which is a string, hence not really of use with the preprocessor. All this however, would not be part of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a valid issue, right now there is no good way to distinguish between different cores since they are essentially seen as different "socs". Im not sure if all am6x r5 devices should have this mpu configuration We will need more discussion surrounding this.
I think for now it should only apply to the AM2434 since it seems it wasn't needed for the other SoCs.
For the Kconfig I did these things 2 commits for splitting them up a bit at least:
siemens@abcfd08
siemens@4f75a54
(the first one splits the defconfig; the second one adds SOC_AM2434_R5F
).
However I also agree we shouldn't increase the scope of this PR further. When this PR is merged I would rebase my current work on main
and start sending PRs, firstly for adding the AM243x LaunchPad which includes these Kconfig changes.
Also for the MPU we should probably also do some changes later so a core can't access the SRAM region of another core.
Edit: We will start upstreaming the MSPI and Flash driver already while this is not merged yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very nice, I feel that we can also split up Kconfig definitions just like you split up defconfigs, this would reduce clutter for different SoCs in the same file, but this is for later, I guess.
Also for the MPU we should probably also do some changes later so a core can't access the SRAM region of another core.
We can, I guess, mask the entire SRAM (all 8 partitions) as no access for all R-cores. The MPU configuration for that core's SRAM partition(s) can then be done in DT, making only that region accessible to that core. This would require moving the sram node from am64x_r5.dtsi
to board/
DTS.
779d955
to
10b3e8b
Compare
Can we remove the Timer label, it is there because I have temporarily added @glneo's dmtimer patch in the PR until it gets merged into main. |
03a4d74
to
e572bc9
Compare
c777478
to
15d80d3
Compare
This driver currently only supports one instance of this timer and uses it as the system clock. The instance is selected by being the first one listed in DT in all places except sys_clock_driver_init() where it uses the node label "systick_timer". This driver should be fixed to correctly support multiple instances of this timer, and the one used for the system timer should be selected based on a flag or alias, not based on label. For now simply use the 0th instance like everywhere else which removes the need to have this node labeled a special way and makes no functional changes to current users. Signed-off-by: Andrew Davis <[email protected]>
Some devices have multiple pinctrl regions; for instance, main pinctrl and mcu pinctrl. Currently there can only be a single pinctrl instance picked form a DT label. This patch makes the pinctrl driver initialise one instance for each node with correct compatible string. Signed-off-by: Amneesh Singh <[email protected]>
This patch adds SoC support and device trees for Texas Instruments AM2434 SoC. Both R5F and M4 cores are supported. Signed-off-by: Amneesh Singh <[email protected]>
minor change: move SRAM MPU configuration from DT's MPU attr to |
This patch reflects changes from the new am64x_m4.dtsi file. Affected boards: - phyboard_electra - sk_am64 Signed-off-by: Amneesh Singh <[email protected]>
This patch adds board support for am2434_evm and am2434 SoC Signed-off-by: Amneesh Singh <[email protected]>
Add documentation for am243x_evm and while at it, add the openocd configuration as well. Co-authored-by: Mika Braunschweig <[email protected]> Signed-off-by: Amneesh Singh <[email protected]>
This patchset aims to: